[AUDIO_WORKLET] Reword API to make it clearer #22741
Conversation
|
Does this not break the public API though? Maybe that is OK since this API is so new? @juj |
It does break it, but it's a few hours old. I cogitated over juj's suggestions after the PR had landed, since the naming also irked me, and thought it worth doing. (It doesn't break any code older than these few hours though, and nothing in a release) |
Oh I see! LGTM then!
|
cwoffenden
force-pushed
the
cw-audio-tweaks-2
branch
from
f6c55da
to
631ade9
Compare
cwoffenden
force-pushed
the
cw-audio-tweaks-2
branch
from
631ade9
to
2efd0f5
Compare
| @@ -117,7 +117,7 @@ function createWasmAudioWorkletProcessor(audioParams) { | |||
| // not have one, so manually copy all bytes in) | |||
| for (i of outputList) { | |||
| for (j of i) { | |||
| for (k = 0; k < this.quantumSize; ++k) { | |||
| for (k = 0; k < this.samplesPerChannel; ++k) { | |||
| j[k] = HEAPF32[outputDataPtr++]; | |||
There was a problem hiding this comment.
Is each sample always an f32?
There was a problem hiding this comment.
In the web API yes, and currently never interleaved. This float by float copy is how it was originally; see my other PR: #22753
cwoffenden
force-pushed
the
cw-audio-tweaks-2
branch
from
00c79dd
to
2efd0f5
Compare
cwoffenden
force-pushed
the
cw-audio-tweaks-2
branch
from
2efd0f5
to
f38b55c
Compare
Following the rest of the renaming.
There was a problem hiding this comment.
LGTM.
I now realize that by parameterizing this, the compiler will lose the ability to optimize a multiplication into a shift. I.e. previously ch*128+i could be turned into a (ch<<7)|i by the compiler, but now it will be a true multiplication and add per each sample.
Maybe that will not be meaningful per-wise, although there is about 2.6 msecs to process each of these callbacks, so people won't want any wasted cycles.. hm...
Anyhow, this rename looks good.
I can rewrite the examples later to work with offsets and some adds. Something like that. |
Following from @juj 's suggestion in #22681, the use of quantum has been reduced to only where it refers to the API. Internally it's named
samplesPerChannelto match thenumberOfChannelsin theAudioSampleFrame.(This is entirely optional and doesn't change anything other than the naming.)